-
Notifications
You must be signed in to change notification settings - Fork 804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hedged request to Store Gateway #6388
Add hedged request to Store Gateway #6388
Conversation
b5378b2
to
3994384
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just a small nit
c595c0f
to
cbd4232
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Only a minor typo!
Signed-off-by: SungJin1212 <[email protected]>
cbd4232
to
dca3507
Compare
hrt.mu.Lock() | ||
err = hrt.TDigest.Add(duration) | ||
if err != nil { | ||
return nil, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential deadlock here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I made a PR here thanos-io/thanos#7962.
cc. @yeya24
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch! @damnever
This PR adds hedged requests to Store Gateway. It can reduce the tail latency between Store Gateway and Object store.
Which issue(s) this PR fixes:
Fixes #6343
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]